Skip to content

fix: remove scope qualifier from Maven/Gradle provider purls#388

Merged
ruromero merged 2 commits into
guacsec:mainfrom
ruromero:TC-3920
Mar 31, 2026
Merged

fix: remove scope qualifier from Maven/Gradle provider purls#388
ruromero merged 2 commits into
guacsec:mainfrom
ruromero:TC-3920

Conversation

@ruromero

@ruromero ruromero commented Mar 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

  • Remove ?scope=compile and ?scope=provided qualifiers from generated purls in Maven and Gradle providers, aligning with the JS client
  • Test-scope filtering now uses DependencyAggregator.isTestDependency() instead of reading from purl qualifiers
  • Updated 34 expected SBOM JSON fixtures to remove scope qualifiers

Implements TC-3920

Test plan

  • All Maven/Gradle/Go provider unit tests pass (285/288, 3 pre-existing GoModulesMainModuleVersion failures)
  • Integration tests pass
  • mvn verify passes

🤖 Generated with Claude Code

@ruromero
ruromero requested a review from soul2zimate March 31, 2026 00:03
@soul2zimate

soul2zimate commented Mar 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Isn't it expected as result of https://redhat.atlassian.net/browse/TC-2752 ？

@soul2zimate

soul2zimate commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

/review

@qodo-code-review

qodo-code-review Bot commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

TC-3920 - Partially compliant

Compliant requirements:

  • Remove ?scope=compile (and any scope qualifier) from PackageURL construction in BaseJavaProvider.DependencyAggregator.toPurl()
  • Remove ?scope=compile (and any scope qualifier) from PackageURL construction in JavaMavenProvider.DependencyAggregator.toPurl()
  • Ensure test-scope dependencies are still filtered out after removing scope qualifiers from purls (filtering no longer relies on purl qualifiers)
  • Update expected SBOM JSON fixtures under src/test/resources/tst_manifests/ to remove scope qualifiers in purl strings

Non-compliant requirements:

  • Verify LicenseUtils.normalizePurlString() still works for license matching after the change (expected to be a no-op regarding qualifiers)

Requires further human verification:

  • Preserve scope information in CycloneDX Component.scope via sbom.addDependency(sourceRef, targetRef, scope) (confirm generated SBOM has correct Component.scope)
  • Ensure formatting conventions are followed (run mvn spotless:apply)
  • Ensure tests and build pass (mvn verify, including JaCoCo thresholds)
  • Maven provider generates purls without scope qualifier (confirm via runtime output / generated SBOM)
  • Gradle provider generates purls without scope qualifier (confirm via runtime output / generated SBOM)
  • Test-scope dependencies remain excluded from SBOMs (confirm via generated SBOM contents)
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

NPE Risk

parseDependencyTree() now relies on DependencyAggregator.isTestDependency() for filtering. Ensure the underlying scope used by isTestDependency() is always initialized (including root lines and any dependency-tree formats where scope might be absent), otherwise scope checks can throw at runtime or incorrectly include/exclude dependencies.

 
int targetDepth = getDepth(target);
while (targetDepth > srcDepth && index < lines.length) {
  if (targetDepth == srcDepth + 1) {
    DependencyAggregator from = parseDep(src);
    DependencyAggregator to = parseDep(target);
    if (!from.isTestDependency() && !to.isTestDependency()) {
      sbom.addDependency(from.toPurl(), to.toPurl(), scope);
    }
Behavior Change

parseDep() now returns a DependencyAggregator instead of a PackageURL. Confirm all call sites were updated accordingly and that no remaining callers still expect a PackageURL (including any subclasses/overrides not shown in this diff), to avoid compile/runtime regressions.

 
DependencyAggregator parseDep(String dep) {
  // root package
  DependencyAggregator dependencyAggregator = new DependencyAggregator();
  // in case line in dependency tree text starts with a letter ( for root artifact).
  if (dep.matches("^\\w.*")) {
    dependencyAggregator = new DependencyAggregator();
    String[] parts = dep.split(":");
    dependencyAggregator.groupId = parts[0];
    dependencyAggregator.artifactId = parts[1];
    dependencyAggregator.version = parts[3];

    return dependencyAggregator;
  }
Logic Validation

Scope qualifiers were removed from toPurl(). Double-check that dependency scope is still consistently propagated to CycloneDX via sbom.addDependency(..., scope) and that the scope variable used there matches the intended dependency’s scope (not accidentally using a parent/line scope).

 
DependencyAggregator from = parseDep(src);
DependencyAggregator to = parseDep(target);
if (!from.isTestDependency() && !to.isTestDependency()) {
  sbom.addDependency(from.toPurl(), to.toPurl(), scope);
}

@ruromero

ruromero commented Mar 31, 2026

Copy link
Copy Markdown
Collaborator Author

Isn't it expected as result of https://redhat.atlassian.net/browse/TC-2752 ？

The linked Jira refers to adding required or optional to the scope attribute.

		{
			"group": "io.quarkus",
			"name": "quarkus-agroal",
			"version": "2.13.5.Final",
			"purl": "pkg:maven/io.quarkus/quarkus-agroal@2.13.5.Final",
			"type": "library",
			"scope": "required",
			"bom-ref": "pkg:maven/io.quarkus/quarkus-agroal@2.13.5.Final"
		},

This PR removes the scope qualifier from the purl

"purl" : "pkg:maven/io.quarkus/quarkus-vertx-http@2.13.5.Final?scope=compile"
...
"purl" : "pkg:maven/io.quarkus/quarkus-vertx-http@2.13.5.Final"

That is not done in any other package manager and in the Javascript client

@soul2zimate

soul2zimate commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

LGTM, but need a rebase.

ruromero and others added 2 commits March 31, 2026 10:22
@ruromero @claude
fix: remove scope qualifier from Maven/Gradle provider purls
542acb3 
Purls no longer contain ?scope=compile or ?scope=provided qualifiers,
aligning with the JS client. Test-scope filtering now uses
DependencyAggregator.isTestDependency() instead of reading from purl
qualifiers. CycloneDX component scope is still set via addDependency().

Implements TC-3920

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ruromero @claude
Remove unused dependencyIsNotTestScope method
c4048cf 
This method became dead code after the TC-3920 refactor that moved
test-scope filtering to use DependencyAggregator.isTestDependency()
directly in parseDependencyTree().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ruromero
ruromero merged commit bbe8c17 into guacsec:main Mar 31, 2026
36 of 39 checks passed
@ruromero
ruromero deleted the TC-3920 branch March 31, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soul2zimate soul2zimate soul2zimate approved these changes

Assignees

No one assigned

Labels

Review effort 3/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ruromero @soul2zimate